Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove what appears to be an un-needed call to self.resolve_redirects when not following redirects as part of creating a request's history. #5492

Merged
merged 1 commit into from
Jun 15, 2020
Merged

Remove what appears to be an un-needed call to self.resolve_redirects when not following redirects as part of creating a request's history. #5492

merged 1 commit into from
Jun 15, 2020

Conversation

dbaxa
Copy link
Contributor

@dbaxa dbaxa commented Jun 15, 2020

No description provided.

@dbaxa
Remove what appears to be an un-needed call to self.resolve_redirects…
fa2dbe9 
… when not following redirects as part of creating a request's history.
@dbaxa dbaxa force-pushed the avoid-calling-resolve_redirects-whe-not-following-redirects branch from 3837895 to fa2dbe9 Compare June 15, 2020 10:17
@sigmavirus24
Copy link
Contributor

At one point, resolve_redirects used to bail early if we weren't following redirects but I guess that changed. Now if there's a redirect to follow, we make the request but we don't use that response which is pretty bad.

Good catch. @nateprewitt @sethmlarson this PR looks good

@dbaxa
Copy link
Contributor Author

dbaxa commented Jun 15, 2020 •
edited
Loading

@sigmavirus24 let me know if you want me to fix up the source branch name and or commit message :-) .

@sigmavirus24
Copy link
Contributor

I'm not a maintainer so not my call. 🙂

nateprewitt
nateprewitt approved these changes Jun 15, 2020
View reviewed changes
Copy link
Member

@nateprewitt nateprewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this is a great catch. Thanks @dbaxa!

@nateprewitt nateprewitt merged commit 2b3436e into psf:master Jun 15, 2020
@nateprewitt nateprewitt mentioned this pull request Jun 17, 2020
Merged
This was referenced Mar 13, 2021
Merged
Merged
Merged
Merged
Merged
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nateprewitt nateprewitt nateprewitt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dbaxa @sigmavirus24 @nateprewitt